Skip to content

Fix missing previous xhardstate - #857

Closed
zerwes wants to merge 4 commits into
Checkmk:masterfrom
zerwes:fix-missing-previousXhardstate
Closed

Fix missing previous xhardstate#857
zerwes wants to merge 4 commits into
Checkmk:masterfrom
zerwes:fix-missing-previousXhardstate

Conversation

@zerwes

@zerwes zerwes commented Sep 22, 2025

Copy link
Copy Markdown
Contributor

see #852 (closed by accident while trying to rebase to force pushes on master using the github UI)

General information

Avoid failing notifications observed in the raw edition after upgrade to 2.4.x

Bug reports

In some cases notifications will fail in the raw edition due to some missing VARs with the error
jinja2.exceptions.UndefinedError: 'dict object' has no attribute 'PREVIOUSSERVICEHARDSTATE'

It seems the bug affects raw and enterprise installations (see #852 (comment))

More details can be found in https://forum.checkmk.com/t/after-upgrade-from-2-3-x-to-2-4-x-raw-sporadically-failing-notifications-with-jinja2-exceptions-undefinederror-dict-object-has-no-attribute-previousservicehardstate/55347

Proposed changes

The proposed changes just assure undefined values for the PREVIOUS[SERVICE|HOST]HARDSTATE in the jinja2 mail notification templates are set to some somehow sane defaults in order to avoid failing notifications.

@github-actions

github-actions Bot commented Oct 7, 2025

Copy link
Copy Markdown

Thank you for your contribution. This pull request has been marked as stale as it has not passed the automated tests and there was no activity for the last 14 days.
Please take a look at the ‘Checks’ section for details on the test results and make the necessary changes.

This pull request will be closed due to inactivity after 60 days, if no action is taken.

@github-actions github-actions Bot added the Stale Scheduled for auto-close label Oct 7, 2025
@zerwes

zerwes commented Oct 7, 2025

Copy link
Copy Markdown
Contributor Author

for the record: even after bringing the PR branch up to date with master, the test fail due to some setup errors, not caused by the changes made in the PR

@github-actions github-actions Bot removed the Stale Scheduled for auto-close label Oct 8, 2025
@floek

floek commented Oct 20, 2025

Copy link
Copy Markdown

Same issue here.

@logan-connolly

Copy link
Copy Markdown
Member

Hi @zerwes,

for the record: even after bringing the PR branch up to date with master, the test fail due to some setup errors, not caused by the changes made in the PR

Yes, I understand. I just spoke with the CI team and I believe the have fixed the issue that was causing this PR to fail. Could you fetch master, rebase atop it, and force push your changes to branch? That way we can see if the GitHub Actions are indeed fixed and a ticket should be generated to work on this PR.

In the meantime, I will download your patch locally and have a look.

Best,
Logan

@zerwes

zerwes commented Oct 21, 2025

Copy link
Copy Markdown
Contributor Author

Hello @logan-connolly
Thank you for your message
As the PR already included a merge commit, I performed again a merge of the master branch into the PR branch. These can be squashed during the merge, if you like.
Tests are running ...

@zerwes

zerwes commented Oct 21, 2025

Copy link
Copy Markdown
Contributor Author

Hm ...
https://github.com/Checkmk/checkmk/actions/runs/18676322078/job/53246961012?pr=857#step:7:73
https://github.com/Checkmk/checkmk/actions/runs/18676322078/job/53246961012?pr=857#step:7:108 ff
failing downloads and no space left on device ... seems the ci/cd is still not yet completely fixed ...

@logan-connolly

Copy link
Copy Markdown
Member

Frustrating... you can see how flakiness in the actions is problematic for generating a ticket :/

But never mind that! I've got your patch locally and am running our test suite on it. When that passes, I'll write up a werk for the fix, push it up to our local infra to test, and then set someone for review.

@logan-connolly

Copy link
Copy Markdown
Member

I have a question regarding the use of "NULL" as the default. Was there a special reason for using "NULL" here?

I think it would make more sense to use "UNKNOWN" as the default since that is a supported state. It will in the end be truncated to "UNKN", but that is okay as it's a common abbreviation for the unknown state in Checkmk.

@zerwes

zerwes commented Oct 21, 2025

Copy link
Copy Markdown
Contributor Author

It will in the end be truncated to "UNKN", but that is okay as it's a common abbreviation for the unknown state in Checkmk

In my opinion NULL is more eloquent and reflecting the missing state better then UNKN, but if you prefer I can change that, it is nothing that will make my heart bleed ... the failing notifications are much more annoying ...

Just let me know if I should update the PR ...

@logan-connolly

Copy link
Copy Markdown
Member

No, nothing to do on your side. I will just update the patch before submitting. Unfortunately, the component owner is on vacation until Friday. I plan to reach out to him regarding this last point. But in the meantime I will queue everything up so that we can just merge on Friday given his decision. Thank you again for your patience!

@logan-connolly

Copy link
Copy Markdown
Member

Hi @zerwes,

I've finally gotten the review and have merged in your contribution. It will take a bit to sync with the GitHub mirror.

Once synced, the commit will be linked to this PR thread and the PR will be closed.

You will be able to shortly see the associated werk under: https://checkmk.com/werk/18927

Thanks again for your contribution :)

@zerwes

zerwes commented Oct 24, 2025

Copy link
Copy Markdown
Contributor Author

Thank you @logan-connolly for your support on this.

CheckmkCI pushed a commit that referenced this pull request Oct 24, 2025
Mail notifications failed when the following variables were undefined:

  - `PREVIOUSHOSTHARDSTATE`
  - `PREVIOUSSERVICEHARDSTATE`

These variables now default to `UNKNOWN` when undefined.

Closes: #857
JIRA-Ref: SUP-26054
Co-authored-by: Logan Connolly <logan.connolly@checkmk.com>
Change-Id: I0809202931f353058ae64415d7bacfc0a792f251
@CheckmkCI CheckmkCI closed this in 018ee42 Oct 24, 2025
@github-actions github-actions Bot locked and limited conversation to collaborators Oct 24, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants